Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally refactor handling of array_length int subclasses in StaticArrayTypeSpec #347

Merged
merged 1 commit into from
May 17, 2022

Conversation

michaeldiamant
Copy link
Contributor

Optionally proposes a refactoring to the question raised in #322 (comment).

Rationale: It's less error prone to centralize the subclass check in StaticArrayTypeSpec rather than have the downstream user remember to cast.

Assumption: It's safe to treat any int subclass as int. The intuition feels reasonable though it should be double checked.

Copy link
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

niceeee!

@tzaffi tzaffi merged commit 00971c4 into blackbox-abi May 17, 2022
@tzaffi
Copy link
Contributor

tzaffi commented May 17, 2022

Optionally proposes a refactoring to the question raised in #322 (comment).

Rationale: It's less error prone to centralize the subclass check in StaticArrayTypeSpec rather than have the downstream user remember to cast.

Assumption: It's safe to treat any int subclass as int. The intuition feels reasonable though it should be double checked.

Subclassing int is an unlikely situation. We should discourage it in this repo except for a really compelling reason.

Ok, I see now that the IntEnum introduced for Address in #289 is a subclass of int

@tzaffi tzaffi deleted the black-abi_casting branch May 17, 2022 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants